Skip to content

haskellPackages.selda-sqlite: unbreak#405820

Closed
rnhmjoj wants to merge 1 commit intoNixOS:haskell-updatesfrom
rnhmjoj:pr-fix-selda-sqlite
Closed

haskellPackages.selda-sqlite: unbreak#405820
rnhmjoj wants to merge 1 commit intoNixOS:haskell-updatesfrom
rnhmjoj:pr-fix-selda-sqlite

Conversation

@rnhmjoj
Copy link
Contributor

@rnhmjoj rnhmjoj commented May 10, 2025

It seems the cabal file of this package is not handled properly by jailbreak-cabal.
It has to be modified manually to remove the constraints.

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 25.05 Release Notes (or backporting 24.11 and 25.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

@github-actions github-actions bot added the 6.topic: haskell General-purpose, statically typed, purely functional programming language label May 10, 2025
@nix-owners nix-owners bot requested a review from maralorn May 10, 2025 08:00
@github-actions github-actions bot added 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. labels May 10, 2025
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have a weak preference for substituteInPlace --replace-fail since that would let us know when to remove this.

FTR, we intentionally leave conditionals alone NixOS/jailbreak-cabal@99eac40.

Copy link
Contributor

@wolfgangwalther wolfgangwalther May 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FTR, we intentionally leave conditionals alone NixOS/jailbreak-cabal@99eac40.

I'm not sure I'm buying the argument made in that commit. The constraints under the flag are required to pick the correct version for a solver - and then to add another peer dependency at the same time for this case.

But none of that applies to us in nixpkgs. The versions we provide are set. The flag configuration is as well. When you hit a constraint and want to jailbreak it, it doesn't matter for us whether it's behind a flag or not.

What am I missing?

Edit: Maybe I am missing some kind of reverse mechanism that sets the flags based on the provided dependencies? I guess I just don't know about this feature.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A flag conditional may, besides restricting the version ranges, set any number of things including e.g. compilation flags. If the flag is marked as automatic, Cabal will attempt to automatically set it based on the preinstalled (by us) versions of packages. If we remove those constraints, it just has to go with the default value of the flag, which may mean incorrect settings and the build failing incorrectly, e.g. because a CPP var isn't set.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have a weak preference for substituteInPlace --replace-fail since that would let us know when to remove this.

It's not possible with substituteInPlace: this code applies to 3 packages and the whitespace is different in each .cabal file, so you need a regex. However we can make sed fail too.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, I guess 3x replace-fail would be easiest to maintain for us (?)

@rnhmjoj rnhmjoj force-pushed the pr-fix-selda-sqlite branch from e000bb5 to cc96b05 Compare May 10, 2025 12:47
@rnhmjoj rnhmjoj requested a review from sternenseemann May 13, 2025 05:50
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's just gonna have the build fail without any message which will cost whoever is going to debug this some time…

@rnhmjoj rnhmjoj force-pushed the pr-fix-selda-sqlite branch from cc96b05 to 6d7682b Compare May 14, 2025 19:40
@sternenseemann
Copy link
Member

Picked as 802c06b. If we were only able to stop reviving these dead projects…

@rnhmjoj
Copy link
Contributor Author

rnhmjoj commented May 15, 2025

If we were only able to stop reviving these dead projects…

Someone is maintaing a fork, perhaps we should switch upstream?

@sternenseemann
Copy link
Member

sternenseemann commented May 15, 2025

I would rather suggest to them doing a Hackage takeover request if they're serious. Their talk of “company specific fixes” is not too confidence inspiring.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

6.topic: haskell General-purpose, statically typed, purely functional programming language 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants